This only seems to happen with debug builds. ==2448==ERROR: AddressSanitizer: access-violation on unknown address 0x000000000148 (pc 0x7ffa509b2f24 bp 0x00ff3c7f50c0 sp 0x00ff3c7f4f00 T0) ==2448==The signal is caused by a READ memory access. ==2448==Hint: address points to the zero page. #0 0x7ffa509b2f23 in nsCSSFrameConstructor::CreateGeneratedContentItem src\layout\base\nsCSSFrameConstructor.cpp:1843 #1 0x7ffa509b8ae2 in nsCSSFrameConstructor::ProcessChildren src\layout\base\nsCSSFrameConstructor.cpp:10150 #2 0x7ffa509ccc26 in nsCSSFrameConstructor::ConstructFrameFromItemInternal src\layout\base\nsCSSFrameConstructor.cpp:3981 #3 0x7ffa509d67f3 in nsCSSFrameConstructor::ConstructFramesFromItem src\layout\base\nsCSSFrameConstructor.cpp:5983 #4 0x7ffa509b7ab9 in nsCSSFrameConstructor::ConstructFramesFromItemList src\layout\base\nsCSSFrameConstructor.cpp:10015 #5 0x7ffa509cfb56 in nsCSSFrameConstructor::ConstructInline src\layout\base\nsCSSFrameConstructor.cpp:11583 #6 0x7ffa509cc189 in nsCSSFrameConstructor::ConstructFrameFromItemInternal src\layout\base\nsCSSFrameConstructor.cpp:3835 #7 0x7ffa509d67f3 in nsCSSFrameConstructor::ConstructFramesFromItem src\layout\base\nsCSSFrameConstructor.cpp:5983 #8 0x7ffa509b7ab9 in nsCSSFrameConstructor::ConstructFramesFromItemList src\layout\base\nsCSSFrameConstructor.cpp:10015 #9 0x7ffa509cfb56 in nsCSSFrameConstructor::ConstructInline src\layout\base\nsCSSFrameConstructor.cpp:11583 #10 0x7ffa509cc189 in nsCSSFrameConstructor::ConstructFrameFromItemInternal src\layout\base\nsCSSFrameConstructor.cpp:3835 #11 0x7ffa509d67f3 in nsCSSFrameConstructor::ConstructFramesFromItem src\layout\base\nsCSSFrameConstructor.cpp:5983 #12 0x7ffa509b7ab9 in nsCSSFrameConstructor::ConstructFramesFromItemList src\layout\base\nsCSSFrameConstructor.cpp:10015 #13 0x7ffa509e4902 in nsCSSFrameConstructor::ContentAppended src\layout\base\nsCSSFrameConstructor.cpp:7179 #14 0x7ffa509619f7 in mozilla::RestyleManager::ProcessRestyledFrames src\layout\base\RestyleManager.cpp:1450 #15 0x7ffa50971686 in mozilla::RestyleManager::DoProcessPendingRestyles src\layout\base\RestyleManager.cpp:3110 #16 0x7ffa5091b3b4 in mozilla::PresShell::DoFlushPendingNotifications src\layout\base\PresShell.cpp:4358 #17 0x7ffa4df7e3a7 in mozilla::EventStateManager::PreHandleEvent src\dom\events\EventStateManager.cpp:690 #18 0x7ffa5094338e in mozilla::PresShell::HandleEventInternal src\layout\base\PresShell.cpp:7701 #19 0x7ffa50940c76 in mozilla::PresShell::HandleEventWithTarget src\layout\base\PresShell.cpp:7520 #20 0x7ffa4e09ec2e in mozilla::PointerEventHandler::DispatchPointerFromMouseOrTouch src\dom\events\PointerEventHandler.cpp:564 #21 0x7ffa5093e931 in mozilla::PresShell::HandleEvent src\layout\base\PresShell.cpp:7297 #22 0x7ffa5010add4 in nsViewManager::DispatchEvent src\view\nsViewManager.cpp:812 #23 0x7ffa5010a6d8 in nsView::HandleEvent src\view\nsView.cpp:1141 #24 0x7ffa5019561c in mozilla::widget::PuppetWidget::DispatchEvent src\widget\PuppetWidget.cpp:408 #25 0x7ffa4a1a8cd8 in mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent src\gfx\layers\apz\util\APZCCallbackHelper.cpp:542 #26 0x7ffa4f8df109 in mozilla::dom::TabChild::HandleRealMouseButtonEvent src\dom\ipc\TabChild.cpp:1659 #27 0x7ffa4f8de557 in mozilla::dom::TabChild::ProcessPendingCoalescedMouseDataAndDispatchEvents src\dom\ipc\TabChild.cpp:1511 #28 0x7ffa508a418c in nsRefreshDriver::Tick src\layout\base\nsRefreshDriver.cpp:1877 #29 0x7ffa508b3bd2 in mozilla::RefreshDriverTimer::TickRefreshDrivers src\layout\base\nsRefreshDriver.cpp:300 #30 0x7ffa508b37ca in mozilla::RefreshDriverTimer::Tick src\layout\base\nsRefreshDriver.cpp:318 #31 0x7ffa508b6c00 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver src\layout\base\nsRefreshDriver.cpp:675 #32 0x7ffa508b6021 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync src\layout\base\nsRefreshDriver.cpp:572 #33 0x7ffa5125e789 in mozilla::layout::VsyncChild::RecvNotify src\layout\ipc\VsyncChild.cpp:76 #34 0x7ffa48de7c4e in mozilla::layout::PVsyncChild::OnMessageReceived src\obj-firefox\ipc\ipdl\PVsyncChild.cpp:167 #35 0x7ffa48b6ff70 in mozilla::ipc::PBackgroundChild::OnMessageReceived src\obj-firefox\ipc\ipdl\PBackgroundChild.cpp:2446 #36 0x7ffa48575aa3 in mozilla::ipc::MessageChannel::DispatchAsyncMessage src\ipc\glue\MessageChannel.cpp:2245 #37 0x7ffa48572c7c in mozilla::ipc::MessageChannel::DispatchMessageW src\ipc\glue\MessageChannel.cpp:2172 #38 0x7ffa4857437d in mozilla::ipc::MessageChannel::RunMessage src\ipc\glue\MessageChannel.cpp:2009 #39 0x7ffa48574831 in mozilla::ipc::MessageChannel::MessageTask::Run src\ipc\glue\MessageChannel.cpp:2042 #40 0x7ffa47253c77 in nsThread::ProcessNextEvent src\xpcom\threads\nsThread.cpp:1244 #41 0x7ffa4725d2e8 in NS_ProcessNextEvent src\xpcom\threads\nsThreadUtils.cpp:530 #42 0x7ffa4857d9e7 in mozilla::ipc::MessagePump::Run src\ipc\glue\MessagePump.cpp:97 #43 0x7ffa484bedcb in MessageLoop::RunInternal src\ipc\chromium\src\base\message_loop.cc:325 #44 0x7ffa484beb37 in MessageLoop::RunHandler src\ipc\chromium\src\base\message_loop.cc:318 #45 0x7ffa484be7d4 in MessageLoop::Run src\ipc\chromium\src\base\message_loop.cc:298 #46 0x7ffa501e1706 in nsBaseAppShell::Run src\widget\nsBaseAppShell.cpp:158 #47 0x7ffa50364b56 in nsAppShell::Run src\widget\windows\nsAppShell.cpp:420 #48 0x7ffa54805e3b in XRE_RunAppShell src\toolkit\xre\nsEmbedFunctions.cpp:961 #49 0x7ffa4857e9da in mozilla::ipc::MessagePumpForChildProcess::Run src\ipc\glue\MessagePump.cpp:269 #50 0x7ffa484bedcb in MessageLoop::RunInternal src\ipc\chromium\src\base\message_loop.cc:325 #51 0x7ffa484beb37 in MessageLoop::RunHandler src\ipc\chromium\src\base\message_loop.cc:318 #52 0x7ffa484be7d4 in MessageLoop::Run src\ipc\chromium\src\base\message_loop.cc:298 #53 0x7ffa548044d5 in XRE_InitChildProcess src\toolkit\xre\nsEmbedFunctions.cpp:787 #54 0x7ff72d052488 in Ordinal0+0x2488 (firefox.exe+0x140002488) #55 0x7ff72d051994 in Ordinal0+0x1994 (firefox.exe+0x140001994) #56 0x7ff72d0514ba in Ordinal0+0x14ba (firefox.exe+0x1400014ba) #57 0x7ff72d18b0cf in TargetNtUnmapViewOfSection+0x25801 (firefox.exe+0x14013b0cf) #58 0x7ffa8aca3033 in BaseThreadInitThunk+0x13 (C:\WINDOWS\System32\KERNEL32.DLL+0x180013033) #59 0x7ffa8d641470 in RtlUserThreadStart+0x20 (C:\WINDOWS\SYSTEM32\ntdll.dll+0x180071470)

https://searchfox.org/mozilla-central/rev/f2028b4c38bff2a50ed6aa1763f6dc5ee62b0cc4/layout/base/nsCSSFrameConstructor.cpp#1843 1843 MOZ_ASSERT(aOriginatingElement.GetShadowRoot()->IsUAWidget()); (gdb) p aOriginatingElement.GetShadowRoot() $1 = (mozilla::dom::ShadowRoot *) 0x0

Hum, I wonder why I didn't encounter this obvious case.

I am also wondering why the original crash test won't hit by this error, given that we will not be attaching Shadow Root without |controls|. https://searchfox.org/mozilla-central/rev/e22c0d152060f4f8d4ca8904094f15f65a1b6f93/layout/generic/crashtests/1493741.html

UA Widget is disabled on 64. It is however enabled on 65. So comment 1 by itself is an easy fix: simply switch to |MOZ_ASSERT_IF(aOriginatingElement.GetShadowRoot(), aOriginatingElement.GetShadowRoot()->IsUAWidget());| will do. But with that the test case hits the old favorite [1]. There is an extra nsTextFrame being inserted into nsVideoFrame for some reason. [1] https://searchfox.org/mozilla-central/rev/e22c0d152060f4f8d4ca8904094f15f65a1b6f93/layout/generic/nsVideoFrame.cpp#407 I'll need to figure out why.

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #4) > So comment 1 by itself is an easy fix: simply switch to > |MOZ_ASSERT_IF(aOriginatingElement.GetShadowRoot(), > aOriginatingElement.GetShadowRoot()->IsUAWidget());| will do. Actually, this is not the right fix. There should always be a shadow root when nsVideoFrame is not a leaf frame... I should look at the DOM to see what's going on.

So the Shadow Root gets unattached because for some reason loading this test case will make us attach, unattach, and reattach the <video> element. Since we are unattaching the shadow root out-of-order with the script runner, we ended up removing the shadow root when the element is bind to the tree... This should tie to bug 1508000 since we only start unattach the shadow root with that bug. !!!!! BindToTree 0x10d682800 UAWidget:1 ComposedDoc:1 !!!!! 0x10d682800, BindToTree AttachAndSetUAShadowRoot !!!!! UnbindFromTree 0x10d682800 UAWidget:1 ComposedDoc:1 !!!!! BindToTree 0x10d682800 UAWidget:1 ComposedDoc:1 !!!!! 0x10d682800, BindToTree AttachAndSetUAShadowRoot !!!!! UnbindFromTree 0x10d682800 UAWidget:1 ComposedDoc:1 !!!!! BindToTree 0x10d682800 UAWidget:1 ComposedDoc:0 !!!!! BindToTree 0x10d682800 UAWidget:1 ComposedDoc:1 !!!!! 0x10d682800, BindToTree AttachAndSetUAShadowRoot !!!!! 0x10d682800 UnbindFromTree UnattachShadow !!!!! 0x10d682800 UnbindFromTree UnattachShadow

I added a bit more analysis in bug 1511130 comment 3. Are you going to look into it? (if so, great!). Just want to avoid duplicating work :)

It looks like the fix is going to be in different places (unbind v.s. bind) and with a different check. I can certainly look into that bug too if you have not invested your time in it. I will propose a fix here soon.

Tracking, just to make sure this lands eventually.

Review comment addressed, please hold off review until I can sure this works. https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b2ab665ff9d84654bbfeee3b863d95d2dd5afc9

The patch now to do what it supposed to do however it causes some fallout outside of the DOM. They seem to be caused by our attempt to clean up the widget and shadow root at the wrong time. https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=215655392&revision=780ae438a10011b8bffb214c23b1416f38a9e631 I am going to try more permutations but basically, the exact cause is beyond my understanding, given that I cannot reproduce them locally either :'(

Yet another attempt. https://treeherder.mozilla.org/#/jobs?repo=try&revision=daa954a2f0fd3caf56b3673b14f717c8a683746c

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #18) ... and that didn't work either.

Ouch, according to bug 1511130 comment 13, the assertion failures are actually caused by the patches there.

Try without bug 1511130 https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d0ba997ee1da502242486f3153c5beff9fae5f5

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #22) This causes the same assertion failure in https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=215843929&repo=try&lineNumber=30640

This should give us the document URI of that broken video element. https://treeherder.mozilla.org/#/jobs?repo=try&revision=1acac41e7e39f7e3440e7d0417327d5e386d4105

So the offending test is actually 1505957.html, in which I replaced the src with srcdoc as a drive-by fix. I probably shouldn't do that :-'(. Anyway, it belongs to another bug. I will fix that crash test by referencing to a real in-tree video file instead. I can still see a few GeckoView test failures though.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a07b29e8eb5f0cf9a0f2200c9aa15186dc1185a It turned out GeckoViewMediaChild.js is listening to UAWidgetBindToTree event directly...

Pushed by tchien@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/adf7dac59077 Do not unattach UA Widget Shadow Root if the element is already re-attached to the tree r=emilio,smaug

Please nominate this for Beta approval when you get a chance.

Comment on attachment 9028799 [details] Bug 1510848 - Do not unattach UA Widget Shadow Root if the element is already re-attached to the tree [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1508000 User impact if declined: None — assertion failure. Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: No Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: None Risk to taking this patch: Medium Why is the change risky/not risky? (and alternatives if risky): This patch corrects the order of the UA Widget construction/destruction & shadow root attachment. It corrects a class of bugs found by fuzzing, but itself could be benefited from staying on Nightly for some time just to make sure we don't end up with another class of bugs identified by fuzzing. In short, should we uplift this? Yes. Is this free of bugs that cannot be addressed in the Beta cycle? I don't know yet. String changes made/needed:

Comment on attachment 9028799 [details] Bug 1510848 - Do not unattach UA Widget Shadow Root if the element is already re-attached to the tree [Triage Comment] Not the most enthusiastic endorsement I've ever seen in an approval request there :P. Anyway, let's get this landed in 65.0b6 now and keep an eye out for any regressions. There's still time in this cycle to backout if it causes more problems than it solves.